-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for building CoreCLR for MacCatalyst/iOS simulator #109928
base: main
Are you sure you want to change the base?
Conversation
@@ -494,7 +494,7 @@ if new_level is -1, the nesting level will not be modified | |||
--*/ | |||
int DBG_change_entrylevel(int new_level); | |||
|
|||
#ifdef __APPLE__ | |||
#ifdef TARGET_OSX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is TARGET_OSX
a subset of TARGET_APPLE
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is.
src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c
Outdated
Show resolved
Hide resolved
src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c
Outdated
Show resolved
Hide resolved
@@ -228,6 +228,9 @@ endif(CLR_CMAKE_TARGET_WIN32) | |||
|
|||
# add the install targets | |||
install_clr(TARGETS coreclr DESTINATIONS . sharedFramework COMPONENT runtime) | |||
if(CLR_CMAKE_HOST_MACCATALYST OR CLR_CMAKE_HOST_IOS) | |||
install_clr(TARGETS coreclr_static DESTINATIONS . sharedFramework COMPONENT runtime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to install this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iOS generally restricts linking to static libraries or dynamic frameworks distributed with the app itself. The aim was to include statically built CoreCLR in the runtime pack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if that means that the single file is the only deployment mechanism on iOS. If it is the case, then I am not sure what would be the scenario where developers would explicitly use the static version of coreclr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linking with a static library typically results in smaller apps, which is why we've always linked Mono statically by default.
Linking dynamically can make the build a little bit faster (from past experience in Xamarin, we never ported this to .NET when we migrated due to time constraints).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The single file is a host statically linked to all the native libraries including coreclr. The scenario when the developers would need static coreclr library would be when they want to use their own host. Thinking about it more, I guess distributing the static coreclr version actually makes sense to enable such scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that sounds like what we want.
Note that .dylib won't do (Apple doesn't allow them in iOS apps), each dynamic library has to be made into a .framework (which works).
FWIW Apple has recommended using no more than 6-8 .frameworks because otherwise it affects startup performance.
@@ -44,7 +44,7 @@ Module Name: | |||
|
|||
#endif // HOST_UNIX | |||
|
|||
#if defined(TARGET_OSX) && defined(HOST_ARM64) && !defined(HAVE_UNW_AARCH64_X19) | |||
#if defined(TARGET_APPLE) && defined(HOST_ARM64) && !defined(HAVE_UNW_AARCH64_X19) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are over 100 usages of __APPLE__
define to identify apple stuff in the PAL. I would prefer staying consistent with that (or change all the usages of the __APPLE__
to TARGET_APPLE
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, there's at least 147 usages of __APPLE__
in CoreCLR vs. 71 of TARGET_APPLE
(in this PR). I can unify it on __APPLE__
if that's preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, keeping uniformity is preferred. In some later PR, we can later change everything to TARGET_APPLE
, but doing it now would unnecessarily grow the already large number of files changed in this PR and do that in places that are unrelated to the gist of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I'll update it.
(may take me a day or two to get to it; currently on sick leave)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -34,6 +34,7 @@ | |||
static const Entry s_sysNative[] = | |||
{ | |||
DllImportEntry(SystemNative_FStat) | |||
#if !defined(TARGET_APPLE) || defined(TARGET_OSX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that we tend to keep this list the same for all platforms and rather handle the missing functionality in the actual implementation of the methods. E.g. for WASI, many of the methods return EINVAL or NULL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is preexisting implementation where it was not done that way. We just never compiled the entrypoints.c on mobile platforms.
Since this is used for corehost which is not particularly useful on mobile platforms maybe I can just drop this part of change and try to exclude corehost build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased after #110092 merge now.
…ld.sh clr+clr.nativeaotlibs+libs+packs -os [iossimulator/maccatalyst/ios] -arch arm64 -cross' Build the singlefilehost even if we don't currently use it Make coreclrhost.h compatible v C(not++) Fix TargetOS::IsMacOS WIP: AppleAppBuilder support for CoreCLR Make the sample app working: dotnet publish src/mono/sample/iOS/Program.csproj -c Release /p:TargetOS=iossimulator /p:TargetArchitecture=arm64 /p:AppleAppBuilderRuntime=CoreCLR /p:DeployAndRun=true /p:UseMonoRuntime=false /p:RunAOTCompilation=false /p:MonoForceInterpreter=false Cleanup Update src/tasks/AppleAppBuilder/AppleAppBuilder.csproj Co-authored-by: Adeel Mujahid <[email protected]> Use sys_icache_invalidate for all iOS-like platforms Consolidate macOS-only entrypoints in a single #if block Apply feedback Fix jitinterface cross-build Infer AppleAppBuilderRuntime from UseMonoRuntime and UseNativeAOTRuntime Minor changes to test infrastructure Add basic documentation Add nearly complete support for building runtime tests Fix building seh-unwind with older iOS SDK Use enum for TargetRuntime in AppleAppBuilder Fix LibraryBuilder build Cleanup Fix after rebase Revert accidental change Fix tvOS build Drop CoreCLR tvOS Simulator support for now
Co-authored-by: Aaron Robinson <[email protected]>
a746372
to
5451afe
Compare
Re-open and rebase of #98127
Build instructions:
./build.sh clr+clr.runtime+libs+packs -os [iossimulator/maccatalyst] -arch [x64/arm64] -cross -c Release
./dotnet.sh publish src/mono/sample/iOS/Program.csproj -c Release /p:TargetOS=maccatalyst /p:TargetArchitecture=arm64 /p:DeployAndRun=true /p:UseMonoRuntime=false /p:RunAOTCompilation=false /p:MonoForceInterpreter=false
Related work:
KnownRuntimeFramework
to use thisNotably, this doesn't include CI scripts to build this or the runtime packs. I am open to suggestions on how to better split this into more digestible/reviewable chunks.